-
Notifications
You must be signed in to change notification settings - Fork 121
[POS Orders] Additional unit tests for created_via
#15848
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[POS Orders] Additional unit tests for created_via
#15848
Conversation
|
|
I was testing p1751454825440359/1751433227.323889-slack-CGPNUU63E similar implementation to Product.prodcutType, and it seems to build at least: 3a11af9 - WDYT? The Yosemite/App layer tests likely need some adjustments from this change. |
jaclync
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests look good, feel free to consider the sales channel move to Networking separately.
| let viewModel = OrderListCellViewModel(order: order, currencySettings: ServiceLocator.currencySettings) | ||
|
|
||
| // Then | ||
| XCTAssertEqual(viewModel.salesChannel, "POS") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is "POS" directly shown in the UI? If so, might we consider making this string localizable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, it does show in the UI but I didn't consider it's a string we'd make localizable, ie: In Spanish the acronym is still POS despite the translation being "punto de venta" 🤔 . I see this is set as localizable string in POSTabViewController thought and definitely makes sense for it to be localized. Updated on 1f93588
Appreciated! Yeah, I'll look into these changes next 👏 |

Continuation of WOOMOB-701
Description
This PR adds additional unit testing to the usage of
created_via. No functional changes have been added.On a related note, I've tried the suggestion on https://github.com/woocommerce/woocommerce-ios/pull/15847/files#r2177500395 about extending the
Ordermodel, but I faced some build troubles which I'm still wondering about. More context here: p1751433227323889-slack-CGPNUU63ETesting information
No functional changes. CI should pass